Skip to content

GH-3719: Fix vectored read allocation limits and fallback safety - #3726

Open
sunchao wants to merge 5 commits into
apache:masterfrom
sunchao:dev/chao/codex/gh-3719-vectored-read-safety
Open

sunchao wants to merge 5 commits into
apache:masterfrom
sunchao:dev/chao/codex/gh-3719-vectored-read-safety

Conversation

@sunchao

@sunchao sunchao commented Aug 16, 2026 •

Copy link
Copy Markdown
Member

Why are the changes needed?

The Hadoop vectored-read path can fall back to ordinary reads after filesystem
requests have been submitted or results partially consumed. Replaying reads into
the same chunk builder can duplicate selected page data and return incorrect
filtered rows. Outstanding requests also need to retain ownership of their stream
and buffers during failure cleanup.

The current path requests a single buffer for an entire contiguous projected
range, without splitting it at parquet.read.allocation.size.

What changes were proposed in this PR?

  • Split requested ranges at the configured allocation size while preserving the
    logical page/column plan. Verify dictionary and V1/V2 page checksums across split
    buffers without another contiguous checksum allocation.
  • Cache file length lazily. A non-interruption metadata lookup failure before
    submission disables vectored IO for that reader and permits ordinary reads.
  • Fail the read after submission is attempted, including synchronous rejection;
    the interface cannot reliably distinguish rejection from partial submission.
  • Share bounded vectored-read workers across readers: 64 by default, configurable
    once through the JVM system property parquet.hadoop.vectored.io.threads.
    Admission, submission, and range waits share one 300-second deadline. Admission
    failure leaves the stream with its reader. Accepted operations retain capacity
    until submission exits, the caller transfers or abandons buffer ownership, and
    failure cleanup finishes. Cancellation cannot free capacity prematurely.
    Idle workers expire after 60 seconds.
  • Track original allocator buffers, including merged and checksum allocations,
    and transfer them to the row-group releaser on success. Preserve automatic
    row-group cleanup on sequential reads and reader closure.
  • Publish available Hadoop futures even when submission fails. Invalidate failed
    readers and defer stream cleanup until submission exits; release buffers only
    when completion can be established. Guarantee abort with finally even when
    draining sibling reads throws an Error, and retain the original failure as
    suppressed on the escaping error. Remove the unused CRC allocator.

The allocation setting bounds requested range lengths, not every filesystem
allocation: backends may merge ranges or align them for checksums and allocate
larger buffers. It does not bound footer/decoder allocations or total memory.
The worker limit does not bound threads created by the filesystem. A backend
that ignores interruption, or a blocked stream close, can retain capacity
indefinitely; when all workers are occupied, further reads time out at admission.
Missing, cancelled, or never-completing result futures can prevent safe buffer
reclamation.

How was this PR tested?

Validated the current implementation on Java 17.0.20.1 with Maven 3.9.16 and
Thrift 0.24.0:

  • Full parquet-hadoop module: 869 tests, zero failures/errors, 26
    Hadoop-capability skips
    with default Hadoop 3.3.0. Spotless passed; RAT
    approved 273 licenses.
  • Focused Hadoop 3.4.1 run with
    -Dparquet.hadoop.vectored.io.threads=2: 110 tests, zero
    failures/errors/skips
    , including the real local-filesystem vectored and
    file-range bridge tests.
  • Regression control: removing only the two new finally guards makes all four
    sibling-error ownership cases fail; the guards were restored before both
    successful runs above.
  • The dependency reactor install, including source/test compilation and API
    compatibility checks, passed. git diff --check passed.

The focused run selected
TestParquetFileReaderVectoredIO,TestParquetFileReaderVectoredOwnership,TestVectoredReadBufferAllocator,TestVectoredReadOperation,TestDataPageChecksums,TestVectorIoBridge,TestFileRangeBridge,TestParquetFileReaderBufferLeak.

New coverage includes saturated admission across repeated retries, capacity
retention during blocked submission and stream closure, interruption before
admission with reader reuse, abort before the worker starts, rejection without
lost capacity, and sibling errors with pending reads and delayed buffer release.
Local setup used a test-JVM hosts file for the machine hostname and interop
fixtures fetched at the commits pinned in the tests. No live S3 validation or
performance benchmark was performed.

Closes #3719.

Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
@sunchao

sunchao commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

cc @wgtmac can you take a look?

Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
Track original filesystem allocations, including checksum buffers and
buffers backing returned slices, and release them with the row group.
Clean up the row group when reading or parsing fails.

Include submission and every requested range in a single read deadline.
Invalidate failed readers and defer stream cleanup until submission exits,
without delaying interruption or recycling buffers still used by IO.

Publish available futures after partial submission failures and fall back
to ordinary reads when a pre-submission file-length lookup fails.

Add regression coverage for buffer ownership, submission failures,
timeouts, interruption, and metadata fallback.
@sunchao
sunchao force-pushed the dev/chao/codex/gh-3719-vectored-read-safety branch from f82eaa4 to 5d3d6f3 Compare September 23, 2026 16:27
Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
@wgtmac

wgtmac commented Sep 24, 2026

Copy link
Copy Markdown
Member

Do you want to take a look as well? @gszadovszky @Fokko @divjotarora

@divjotarora

Copy link
Copy Markdown
Contributor

@wgtmac I'd be interested in reviewing, but I'm a bit swamped right now. Can get to it tomorrow if that's OK

@gszadovszky gszadovszky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@divjotarora divjotarora left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, thanks for the investigation and thorough fix @sunchao!

Comment thread parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vectored Parquet reads can fall back unsafely after partial asynchronous reads and exceed allocation limits

5 participants